Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix failure when using capital letters in image alias in 'FROM ... AS…' instruction #839

Merged
merged 1 commit into from
Dec 9, 2019
Merged

Fix failure when using capital letters in image alias in 'FROM ... AS…' instruction #839

merged 1 commit into from
Dec 9, 2019

Conversation

antechrestos
Copy link
Contributor

@antechrestos antechrestos commented Oct 26, 2019 •
edited
Loading

Fixes #770
Fixes #592

Description

The third library moby/buildkit lowers the image alias used in FROM image AS alias instruction.
This change fixes this issue by making the resolution of dependencies agnostic to text case.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes unit tests
  • Adds integration tests if needed(not needed to my mind as unit tests cover this fix).

See the contribution guide for more details.

Reviewer Notes

  • The code flow looks good.
  • Unit tests and or integration tests added.

Release Notes

Given a user use a mounting instruction such as FROM my-image as myImage, when the users reference this image from a command "COPY -- from myImage", kaniko currently fails. This fix aims to make kaniko agnostic to letter case as it is the case of the vendor library moby/buildkit.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@antechrestos
Copy link
Contributor Author

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@antechrestos antechrestos force-pushed the fix/upcase_for_from_stage_name branch 2 times, most recently from 51aa89c to eff63b4 Compare November 1, 2019 16:07
@antechrestos antechrestos force-pushed the fix/upcase_for_from_stage_name branch 2 times, most recently from 9f272b0 to 2e6b8ff Compare November 7, 2019 17:45
@antechrestos
Copy link
Contributor Author

antechrestos commented Nov 9, 2019 •
edited
Loading

@tejal29 Apparently the integration tests are failing. Yet I cannot access the details link. No matter I read the integration tests, I have not figured which of them fails as none of them uses a capital letter in its COPY -- from instruction

Can you share the failure as I don't have a GCS bucket.

PS:

I fixed the link
http://fusion/runanalysis/info/prod%3Aruntimes-common%2Fkaniko%2Fpresubmit/prod%3Aruntimes-common%2Fkaniko%2Fpresubmit/KOKORO/60fbcd60-3ea1-4248-a7ca-d8dccac51a0a/0/prod%3Aruntimes-common%2Fkaniko%2Fpresubmit/Targets

By replacing it with

http://fusion.corp.google.com/runanalysis/info/prod%3Aruntimes-common%2Fkaniko%2Fpresubmit/prod%3Aruntimes-common%2Fkaniko%2Fpresubmit/KOKORO/60fbcd60-3ea1-4248-a7ca-d8dccac51a0a/0/prod%3Aruntimes-common%2Fkaniko%2Fpresubmit/Targets

Yet I do not have a login 😄

@antechrestos antechrestos mentioned this pull request Nov 13, 2019
Closed
@antechrestos
Copy link
Contributor Author

antechrestos commented Nov 13, 2019 •
edited
Loading

@tejal29 don't know if previous kokoro launch ended with the same output but locally I have difficulties launching integration tests

@antechrestos
Copy link
Contributor Author

@tejal29 @cvgw Is it possible to trigger a kokoro check? I cannot test it from my computer as I reported before.... 😢

@antechrestos
Copy link
Contributor Author

@cvgw Thank you Is it possible to get the output?

Thank you by advance :)

@cvgw
Copy link
Contributor

cvgw commented Nov 25, 2019

@antechrestos looks like one test failed

--- FAIL: TestRelativePaths (0.00s)
    --- FAIL: TestRelativePaths/test_relative_Dockerfile_test_copy (7.00s)
        integration_test.go:406: diff = [
              {
                "Image1": "gcr.io/kaniko-test/docker-dockerfile_test_copy",
                "Image2": "gcr.io/kaniko-test/kaniko-dockerfile_test_copy",
                "DiffType": "File",
                "Diff": {
                  "Adds": [
                    {
                      "Name": "/dir/cache",
                      "Size": 0
                    },
                    {
                      "Name": "/newdir/cache",
                      "Size": 0
                    }
                  ],
                  "Dels": [
                    {
                      "Name": "/dir/dockerfiles/Dockerfile_test_add_dest_symlink_dir",
                      "Size": 61
                    },
                    {
                      "Name": "/newdir/dockerfiles/Dockerfile_test_add_dest_symlink_dir",
                      "Size": 61
                    }
                  ],
                  "Mods": null
                }
              },
              {
                "Image1": "gcr.io/kaniko-test/docker-dockerfile_test_copy",
                "Image2": "gcr.io/kaniko-test/kaniko-dockerfile_test_copy",
                "DiffType": "Metadata",
                "Diff": {
                  "Adds": [],
                  "Dels": []
                }
              }
            ]
        integration_test.go:409: []integration.diffOutput differ (-got, +want): {[]integration.diffOutput}[0].Diff.Adds:
            	-: []integration.fileDiff{{Name: "/dir/cache"}, {Name: "/newdir/cache"}}
            	+: []integration.fileDiff(nil)
            {[]integration.diffOutput}[0].Diff.Dels:
            	-: []integration.fileDiff{{Name: "/dir/dockerfiles/Dockerfile_test_add_dest_symlink_dir", Size: 61}, {Name: "/newdir/dockerfiles/Dockerfile_test_add_dest_symlink_dir", Size: 61}}
            	+: []integration.fileDiff(nil)
FAIL

@antechrestos
Copy link
Contributor Author

@cvgw Thank you 😄
Now I am puzzled 🤔

@antechrestos
Copy link
Contributor Author

@cvgw I launched the failed test on my laptop and it passed

❯go test -v --bucket $GCS_BUCKET --repo $IMAGE_REPO -test.run RelativePaths
2019/11/29 00:42:36 Creating tarball of integration test files to use as build context
2019/11/29 00:42:36 Uploading file at /tmp/buce8373/788886540/context_1574984556922350732.tar.gz to GCS bucket at gs://antechrestos-test-kaniko//tmp/buce8373/788886540/context_1574984556922350732.tar.gz
Building kaniko image
Building cache warmer image
Building onbuild base image
Pushing onbuild base image
Building hardlink base image
Pushing hardlink base image
...
=== RUN   TestRelativePaths
=== RUN   TestRelativePaths/test_relative_Dockerfile_test_copy
=== PAUSE TestRelativePaths/test_relative_Dockerfile_test_copy
=== CONT  TestRelativePaths/test_relative_Dockerfile_test_copy
--- PASS: TestRelativePaths (0.00s)
    --- PASS: TestRelativePaths/test_relative_Dockerfile_test_copy (13.47s)
        integration_test.go:406: diff = [
              {
                "Image1": "gcr.io/silent-caster-258821/antechrestos-test-kaniko/docker-dockerfile_test_copy",
                "Image2": "gcr.io/silent-caster-258821/antechrestos-test-kaniko/kaniko-dockerfile_test_copy",
                "DiffType": "File",
                "Diff": {
                  "Adds": null,
                  "Dels": null,
                  "Mods": null
                }
              },
              {
                "Image1": "gcr.io/silent-caster-258821/antechrestos-test-kaniko/docker-dockerfile_test_copy",
                "Image2": "gcr.io/silent-caster-258821/antechrestos-test-kaniko/kaniko-dockerfile_test_copy",
                "DiffType": "Metadata",
                "Diff": {
                  "Adds": [],
                  "Dels": []
                }
              }
            ]
PASS
ok  	github.com/GoogleContainerTools/kaniko/integration	360.027s
❯ git log --oneline | head -1
8e0a566 Fix failure when using capital letters in image alias in 'FROM ... AS' instruction

What can make the result differ?

Thanks

@antechrestos
Fix failure when using capital letters in image alias in 'FROM ... AS…
d22a760 
…' instruction

The third library moby/buildkit lowers the image alias used in 'FROM .. AS' instruction.
This change fixes this issue by making the resolve of dependencies agnostic to case.

Fixes #592
Fixes #770
@antechrestos
Copy link
Contributor Author

antechrestos commented Dec 6, 2019 •
edited
Loading

@cvgw I made it, my build passed! 😃
I also rebased my code on master branch

cvgw
cvgw approved these changes Dec 6, 2019
View reviewed changes
Copy link
Contributor

@cvgw cvgw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @antechrestos, looks good!

@tejal29 tejal29 merged commit acb5b9f into GoogleContainerTools:master Dec 9, 2019
@antechrestos antechrestos deleted the fix/upcase_for_from_stage_name branch January 3, 2020 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cvgw cvgw cvgw approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Build fails when FROM image AS alias has capital letters camelCase stage names do not work
5 participants
@antechrestos @googlebot @cvgw @tejal29 @kokoro-team